-
Notifications
You must be signed in to change notification settings - Fork 3
Change resource metadata load and access behavior #127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| } | ||
|
|
||
| @cached_property | ||
| def bibliography(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very specific to the echemd metadata schema. So I moved it from the parent collection.
| return f"Echemdb({self.identifier!r})" | ||
|
|
||
| @property | ||
| def bibliography(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. This is an echedb specific method moved from the base entry.
|
|
||
| >>> entry = Entry.create_examples()[0] | ||
| >>> entry.source # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE | ||
| >>> entry.echemdb.source # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously the default metadata key was echemd, which is basically only a good idea for the echemdb module.
|
|
||
| return entry | ||
|
|
||
| def remove_columns(self, *field_names): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had to add this for the electrochemistry-data repository to convert raw data
| @@ -693,58 +824,105 @@ def plot(self, x_label=None, y_label=None, name=None): | |||
| ) | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
following lines fix #128
|
|
||
| @classmethod | ||
| def from_csv(cls, csvname, metadata=None, fields=None): | ||
| def update_fields(self, fields): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in a next iteration I will have to fix how we handle the modification of the fiels. It feels really cluttered. There should be a method which says update_field(fieldname, key, value), which is more intuitive.
| return self | ||
|
|
||
| @classmethod | ||
| def from_csv( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the method now only contains arguments related to the CSV
| self.resource.name = basename | ||
|
|
||
| # convert a pandas resource into a csv resource | ||
| if self.resource.format == "pandas": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to verify if this is the exact structure for a saved csv.
| # This file is part of unitpackage. | ||
| # | ||
| # Copyright (C) 2021-2025 Albert Engstfeld | ||
| # Copyright (C) 2021-2026 Albert Engstfeld |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file I split the methods more carefully and provided more meaningful names.
| if fields: | ||
| # Update fields in the Resource describing the data in the CSV | ||
| resource_schema = resource.schema | ||
| def validate_field_structure(fields): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might change in the future in an overhaul of the field handling mentioned above
Checklist
doc/news/.Addresses #20
Depends on #126
Fixes #128